Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract Zlib/Brotli custom allocator #2705

Merged
merged 1 commit into from
Sep 13, 2024
Merged

Extract Zlib/Brotli custom allocator #2705

merged 1 commit into from
Sep 13, 2024

Conversation

npaun
Copy link
Member

@npaun npaun commented Sep 12, 2024

  • Zlib and Brotli use malloc and free by default to manage memory. If they leak, it's an upstream bug.
  • However, they also allow for custom allocators to be passed.
  • This PR extracts our custom allocator functions AllocForZlib, AllocForBrotli, FreeForZlib into a new class called Allocator.
  • Allocator is now used for both the streaming and one-shot implementations.
  • I dropped the stuffing of size into the allocated regions because the backing kj::heapArrays are already sized objects.
  • At present, the information collected by allocator is not used. However, we could block excessively large allocations, and report allocations to V8 in the future.
  • Our current implementation is a hash map of pointers, but what we'd really like is a kind of arena allocator, in my opinion at least.

Closes #2678

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have @anonrig review also

@anonrig anonrig merged commit 13f843a into main Sep 13, 2024
13 checks passed
@anonrig anonrig deleted the npaun/zlib-allocator branch September 13, 2024 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track brotli allocations
3 participants